roborev: Combined Review (9404951)

Verdict: The
pull request successfully implements a generic JSON webhook integration, but requires a fix for a medium-severity credential exposure vulnerability in error logs.

Medium

• Credential exposure in webhook error logs
  • Files: [internal/daemon/hooks.go:292](/home/roborev/repos/rob
    orev/internal/daemon/hooks.go:292), hooks.go:298, [hooks.go:306](/home/roborev/repos
    /roborev/internal/daemon/hooks.go:306), hooks.go:314, [hooks.go:317](/home/roborev
    /repos/roborev/internal/daemon/hooks.go:317)
  • Description: The code logs url=%q on webhook failures. If users configure webhook credentials in URL userinfo or query params (for example, https://token@host/... or ? api_key=...), those secrets are written to logs.
  • Suggested remediation: Parse and redact URLs before logging (strip User, query string, and fragments), or log only scheme + host + path stem.

Synthesized from 3 reviews (agents: codex, gemini | types: default, security)

roborev: Combined Review (b89b364)

Verdict: The PR successfully adds webhook functionality, but contains a
medium-severity security issue where partial URL redaction may leak secret tokens in daemon logs.

Medium

• Incomplete Webhook URL Redaction Leaks Secrets
  • File: [internal/daemon/hooks.go:337](/home/roborev/repos/roborev/internal
    /daemon/hooks.go:337)
  • Issue: The redactWebhookURL function preserves the first path segment of the URL. For webhook URLs where the secret is passed as a single path segment (e.g., https://host/<secret>), this logic fails to mask
    the token and causes the credential to be fully exposed in the daemon logs during an error.
  • Suggested Fix: Redact the entire path unconditionally (e.g., always output /<redacted> or /... when the path is non-empty) rather than preserving any literal path segment. Do
    not preserve any segment unless explicitly proven non-sensitive via an allowlist.

Synthesized from 3 reviews (agents: codex, gemini | types: default, security)

roborev: Combined Review (faf7808)

Verdict: The webhook implementation introduces a medium-severity vulnerability by logging untrusted response bodies, which could leak secrets or allow log forging.

Medium

• Raw webhook error bodies can leak secrets and allow log forging

  • File: internal/daemon/hooks.go:319-320
  • Description: On non-2xx responses, post Webhook logs up to 1KB of the raw, untrusted remote response body verbatim. Even though the configured URL is redacted, a malicious or compromised endpoint can inject newline/control characters into logs (log forging) or reflect sensitive request details (including the tokenized URL path/query) back into the logs, bypassing the
    URL-redaction goal.
  • Suggested Remediation: Do not log raw response bodies by default. Log only the status code (safest), or sanitize/escape body content to a strict single-line printable format and run secret redaction before logging. Keep detailed body logging behind an explicit debug flag
    .
  • Testing Gap: Add a test where the webhook server returns a body containing the original secret path/token and assert the logs do not contain it.

Synthesized from 3 reviews (agents: codex, gemini | types: default, security)

roborev: Combined Review (591f13a)

Verdict: The PR adds webhook support, but introduces medium-severity issues around credential exposure in config
/logs and missing basic auth headers.

Medium

• Sensitive webhook URLs exposed by config list

  • Files: internal/config/keyval.go:740, cmd/roborev/config_cmd.go:377, internal/ config/keyval_test.go:351
  • Description: Masking checks exact keys (e.g., hooks.url), but config flattening emits hooks as a single hooks=<slice> value. The formatted slice includes full struct contents, allowing webhook URLs to appear unmasked in
    CLI output.
  • Suggested Fix: Flatten slice-of-struct fields into child keys (e.g., hooks.0.url) and mask by sensitive tag, or conservatively treat the parent hooks as sensitive when it contains sensitive descendants.

• Raw webhook error responses may expose credentials

  • File: internal/daemon/hooks.go:321
  • Description: In postWebhook, non-2xx responses log up to 1024 bytes of the raw response body. External services often echo the request path/query in error responses. If the webhook
    URL contains secret tokens, this can re-expose credentials, defeating URL redaction.
  • Suggested Fix: Do not log raw webhook response bodies by default. Log status only, or log a sanitized/redacted body behind an explicit debug flag.

• Basic auth in webhook URLs is ignored

  ---

File:** internal/daemon/hooks.go (~line 301)

• Description: Go's http.NewRequest does not automatically convert URL userinfo into an Authorization header. Webhooks configured with basic auth in the URL (e.g., https://user:pass @example.com/) will be sent without credentials and likely fail with a 401 Unauthorized.
• Suggested Fix: Extract credentials and explicitly set them on the request: if req.URL.User != nil { pwd, _ := req.URL.User.Password(); req.Set BasicAuth(req.URL.User.Username(), pwd) }

Synthesized from 3 reviews (agents: codex, gemini | types: default, security)